Skip to content

feat: Improve namespace validation logic and enhanced inspect output #4618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

feat: Improve namespace validation logic and enhanced inspect output #4618

wants to merge 6 commits into from

Conversation

@opjt
Copy link
Contributor

@opjt opjt commented Nov 25, 2025 •
edited
Loading

related issue #3851

@AkihiroSuda

changes

fixes

  • Added validation to require the --label flag for the update command.
  • Added namespace existence checks to both the update and inspect commands.
    • (inspect) If non-existent namespaces are included, a warning log is now emitted.
      • this logic is based on the behaviorvolume-inspect

features

  • Enhanced namespace inspect output:
    • Added Containers information (count, ids)
    • Added Images information (count, ids)
    • Added Volumes information (count, names)
  • The inspect command now provides the same level of information available in namespace list.

@opjt opjt force-pushed the fix/namespace-issue branch 2 times, most recently from a34a665 to 21163f0 Compare November 25, 2025 23:45
@AkihiroSuda AkihiroSuda added this to the v2.2.1 (?) milestone Nov 27, 2025
@AkihiroSuda AkihiroSuda requested a review from a team November 28, 2025 07:53
@Shubhranshu153
Copy link
Contributor

Shubhranshu153 commented Nov 29, 2025

NIT: Squash the commits. (Probably 2 commits, one for the feature and one for the fix)

@opjt opjt changed the title fix: Improve namespace validation logic for update, inspect command feat: Improve namespace validation logic and enhanced inspect output Nov 29, 2025
@opjt opjt force-pushed the fix/namespace-issue branch from 2a7ac0e to 0a3ac58 Compare November 29, 2025 02:37
Shubhranshu153
Shubhranshu153 reviewed Nov 29, 2025
View reviewed changes
Shubhranshu153
Shubhranshu153 reviewed Nov 29, 2025
View reviewed changes
@opjt opjt force-pushed the fix/namespace-issue branch from a5d875c to 5ea5532 Compare November 29, 2025 09:47
@opjt opjt force-pushed the fix/namespace-issue branch from a7a7972 to e1b84ae Compare November 29, 2025 10:26
Shubhranshu153
Shubhranshu153 reviewed Nov 30, 2025
View reviewed changes
@opjt
Copy link
Contributor Author

opjt commented Dec 1, 2025

@AkihiroSuda When you have time, could you please take a look at this PR?

AkihiroSuda
AkihiroSuda reviewed Dec 2, 2025
View reviewed changes
pkg/inspecttypes/native/namespace.go
Copy link
Member

@AkihiroSuda AkihiroSuda Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why include IDs?

AkihiroSuda
AkihiroSuda reviewed Dec 2, 2025
View reviewed changes
pkg/cmd/namespace/inspect.go
Copy link
Member

@AkihiroSuda AkihiroSuda Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change will make inspect slower, no?

@AkihiroSuda
Copy link
Member

AkihiroSuda commented Dec 2, 2025

features

This seems overloaded to this PR recently?
I suggest splitting this to another PR for further discussion.
By default, the namespace operations should remain lightweight.

@opjt
Copy link
Contributor Author

opjt commented Dec 2, 2025

Thank you for the suggestion.

Regarding namespace inspect, I agree that it should contain more details, aligning with the discussion in 3851 . Is it acceptable to omit the IDs information in the output?

I also agree that this PR is currently overloaded with a mix of fixes and features, which makes it hard to review. I will split the changes into separate, more focused PRs for better reviewability and resubmit them shortly.

@opjt opjt closed this Dec 2, 2025
@AkihiroSuda AkihiroSuda removed this from the v2.2.1 (?) milestone Dec 2, 2025
@opjt opjt mentioned this pull request Dec 2, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AkihiroSuda AkihiroSuda AkihiroSuda left review comments

+1 more reviewer

@Shubhranshu153 Shubhranshu153 Shubhranshu153 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@opjt @Shubhranshu153 @AkihiroSuda